-
Notifications
You must be signed in to change notification settings - Fork 71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[3.2] Create a simple benchmarking framework and use it to benchmark crypto primitives #17
Conversation
…ives Create a generic micro-benchmarking framework intended for any functions. As the first application, alt_bn_128 adding/multiplying/pairing, sha3-256/keccak256 hashing, k1/r1/webauthn key signing/recovering, and modexp are benchmarked
} | ||
|
||
void printt_header() { | ||
std::cout << std::left << std::setw(name_width) << "function" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this supposed to be printt_header
or print_header
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your sharp eyes! Sorry for this copy/paste error.
benchmark/key.cpp
Outdated
auto key = private_key::generate<ecc::private_key_shim>(); | ||
|
||
auto sign_f = [&]() { | ||
key.sign(digest); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would recommend disabling the canonical requirement for the signing. Otherwise the value could swing widely based on the private key you happened to randomly generate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! I will change to use fixed private keys so we can compare the values consistently across different runs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you just pass false here for the second parameter, it'll be constant time for all private keys anyways
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I am benchmarking both canonical and non-canonical cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
imo there is very little utility in the canonical case -- I would never look at it personally. It's dependent on both the private key and payload, both of which are fixed here, thus making the time spent always be some constant multiplied with the non-canonical time. Up to you though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will remove the canonical case.
private key of k1 was randomly generated. It caused big swing in time with canonical requirement. This commit makes the private key fixed and benchmarks both canonical and non-canonical cases
In general LGTM but, in some conversation somewhere, didn't we think we wanted coverage of all hashes accessible via host functions? If anything, sha256 seems like a must have given its broad usage across the entire protocol. |
I'll benchmark all crypto primitives which are exposed as host functions. |
Make sure all exceptions are caught when executing code in http_plugin's thread pool, so Leap doesn't crash when processing APIs.
Transition to a temporary beta license
Resolve eosnetworkfoundation/mandel#786.
This PR is built upon eosnetworkfoundation/mandel#799. Changes to incorporate the comments from that PR (eosnetworkfoundation/mandel#799) are
sha1, sha256, sha512, ripemd160, sha3-256, keccak256
hashing,k1/r1/webauthn
key signing/recovering, andblake2
std::chrono::high_resolution_clock
${CMAKE_CURRENT_SOURCE_DIR}
inCMakeList.txt
JSON format output will be done in a future improvement task.
Help
Benchmarking all functions
Benchmarking a single feature with a different number of repeats from the default 1,000